Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression with HasImport/HasExample data fields, and (finally) document template data fields #162

Merged
merged 5 commits into from Jun 29, 2022

Conversation

detro
Copy link
Contributor

@detro detro commented Jun 29, 2022

This is a regression from #155: unfortunately the logic didn't check that the file where an example/import was expected, actually existed.

Additionally, given I had to spend time figuring out a bit more how those data fields are populated, I caught the opportunity to finally document the data fields.

As a bonus, I added 3 template functions that were very low hanging fruits: lower, upper and title.

@detro detro requested a review from a team as a code owner June 29, 2022 13:15
@bflad
Copy link
Member

bflad commented Jun 29, 2022

Should a similar change be made to the provider and data source template rendering fields and is there a way to unit test this?

@detro
Copy link
Contributor Author

detro commented Jun 29, 2022

We totally need unit tests and regression tests for this stuff :(

@detro
Copy link
Contributor Author

detro commented Jun 29, 2022

I'm adding the same fix to the Provider section too.

The data struct for Resource and Data Sources is the same: yet another thing that should be refactored.

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. LGTM

@detro detro merged commit fac6290 into main Jun 29, 2022
@detro detro deleted the detro/document_template_data_fields branch June 29, 2022 15:35
@jacobbednarz
Copy link
Contributor

gah, of course 🤦 thanks for finding this @detro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants